src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java (2)

410-437: Add explicit precondition checks for required UUIDs.

groupUuid and modificationUuid are mandatory for URI expansion; guard them at method entry to fail fast with a clear contract.

♻️ Proposed change

 public void moveSubModification(
         UUID groupUuid,
         UUID sourceCompositeUuid,
         UUID targetCompositeUuid,
         UUID modificationUuid,
         UUID beforeUuid) {
+    Objects.requireNonNull(groupUuid, "groupUuid is required");
+    Objects.requireNonNull(modificationUuid, "modificationUuid is required");
 
     var path = UriComponentsBuilder.fromPath(
             COMPOSITE_PATH + "groups" + DELIMITER
                     + "{groupUuid}" + DELIMITER
                     + "sub-modifications" + DELIMITER
                     + "{modificationUuid}");

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java`
around lines 410 - 437, The method moveSubModification should validate required
UUID parameters up front: add explicit null-check preconditions for groupUuid
and modificationUuid (used in URI expansion) at the start of moveSubModification
and throw a clear IllegalArgumentException (or use Objects.requireNonNull) with
descriptive messages; keep existing optional handling for sourceCompositeUuid,
targetCompositeUuid, and beforeUuid unchanged so the rest of the method can
assume groupUuid and modificationUuid are non-null when building the path and
calling restTemplate.put.

---

439-450: Short-circuit empty input and return a null-safe set.

For an empty list, this can return immediately without a remote call; also avoid leaking a null body to callers.

♻️ Proposed change

 public Set<UUID> expandToLeafUuids(List<UUID> modificationUuids) {
+    Objects.requireNonNull(modificationUuids, "modificationUuids is required");
+    if (modificationUuids.isEmpty()) {
+        return Set.of();
+    }
+
     var path = UriComponentsBuilder.fromPath(COMPOSITE_PATH + "leaf-uuids")
             .queryParam("uuids", modificationUuids)
             .toUriString();
 
-    return restTemplate.exchange(
+    Set<UUID> body = restTemplate.exchange(
             getNetworkModificationServerURI(false) + path,
             HttpMethod.GET,
             null,
             new ParameterizedTypeReference<Set<UUID>>() { }
-    ).getBody();
+    ).getBody();
+    return body != null ? body : Set.of();
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@src/main/java/org/gridsuite/study/server/service/NetworkModificationService.java`
around lines 439 - 450, The expandToLeafUuids method should short-circuit when
modificationUuids is empty and always return a non-null Set to callers; change
expandToLeafUuids to check modificationUuids.isEmpty() and return
Collections.emptySet() (or Set.of()) immediately instead of calling
restTemplate.exchange, and after the remote call ensure the result from
restTemplate.exchange(...).getBody() is null-checked and replaced with an empty
Set before returning; reference expandToLeafUuids, COMPOSITE_PATH,
restTemplate.exchange and getNetworkModificationServerURI to locate the changes.

src/test/java/org/gridsuite/study/server/NetworkModificationTest.java (2)

3138-3139: Make move-case query assertions strict (including absent params).

Line 3138 and Line 3154 only assert required params, so these cases can still pass if unwanted query params are forwarded. Assert missing params explicitly.

♻️ Proposed test hardening

-        WireMockUtilsCriteria.verifyPutRequest(wireMockServer, moveSubModifUrlPattern, false, Map.of(
-                "targetCompositeUuid", WireMock.equalTo(targetCompositeUuid.toString())), null);
+        WireMockUtilsCriteria.verifyPutRequest(wireMockServer, moveSubModifUrlPattern, false, Map.of(
+                "targetCompositeUuid", WireMock.equalTo(targetCompositeUuid.toString()),
+                "sourceCompositeUuid", WireMock.absent(),
+                "beforeUuid", WireMock.absent()), null);

...
-        WireMockUtilsCriteria.verifyPutRequest(wireMockServer, moveSubModifUrlPattern, false, Map.of(
-                "sourceCompositeUuid", WireMock.equalTo(sourceCompositeUuid.toString())), null);
+        WireMockUtilsCriteria.verifyPutRequest(wireMockServer, moveSubModifUrlPattern, false, Map.of(
+                "sourceCompositeUuid", WireMock.equalTo(sourceCompositeUuid.toString()),
+                "targetCompositeUuid", WireMock.absent(),
+                "beforeUuid", WireMock.absent()), null);

Also applies to: 3154-3155

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/gridsuite/study/server/NetworkModificationTest.java` around
lines 3138 - 3139, The verifyPutRequest call is only checking required params
and allows unwanted query params to slip through; update the
WireMockUtilsCriteria.verifyPutRequest invocations for moveSubModifUrlPattern
(the call using targetCompositeUuid) and the similar call later to assert absent
params explicitly by adding the expected absent query keys to the Map of
matchers using WireMock.absent() (or the project's equivalent absent matcher)
for keys that must not be forwarded, while keeping
WireMock.equalTo(targetCompositeUuid.toString()) for the required
"targetCompositeUuid".

---

3211-3216: Assert exact exclusion UUID set, not a superset.

Line 3214 currently accepts any superset. Use exact-set equality so the test fails when unexpected UUIDs are included.

♻️ Proposed assertion tightening

+        Set<UUID> expectedExcluded = new HashSet<>(expandedLeafUuids);
+        expectedExcluded.add(compositeUuid);
         verify(rootNetworkNodeInfoService, times(1)).updateModificationsToExclude(
                 eq(nodeUuid),
                 eq(rootNetworkUuid),
-                argThat((Set<UUID> uuids) -> uuids.contains(compositeUuid)
-                        && uuids.containsAll(expandedLeafUuids)),
+                argThat(expectedExcluded::equals),
                 eq(false));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/test/java/org/gridsuite/study/server/NetworkModificationTest.java` around
lines 3211 - 3216, The test currently allows a superset because argThat checks
contains/composite and containsAll; change the assertion in
NetworkModificationTest so the UUID set is compared for exact equality instead
of subset/superset checks: construct the expected Set<UUID> containing
compositeUuid plus expandedLeafUuids and replace argThat((Set<UUID> uuids) ->
uuids.contains(compositeUuid) && uuids.containsAll(expandedLeafUuids)) with an
argument matcher that calls equals(expectedSet) (or use eq(expectedSet)) when
verifying rootNetworkNodeInfoService.updateModificationsToExclude(...), so
unexpected UUIDs will fail the test.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes